Skip to content

rm: overriding the libc call when implementation does not match gnu coreutils exactly #9604

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
ChrisDryden wants to merge 4 commits into
base: main
Choose a base branch
from
Open

rm: overriding the libc call when implementation does not match gnu coreutils exactly #9604

ChrisDryden wants to merge 4 commits into from

Conversation

@ChrisDryden
Copy link
Collaborator

@ChrisDryden ChrisDryden commented Dec 8, 2025 •
edited
Loading

This was originally intended to be a demonstration for how we could modify the integration tests to recognize the libc calls we were using.

I discovered that I could wrap the implementation in this helper method to be able to address this tests in a relatively readable way compared to modifying the original code.

As it stands I believe there are three skipped tests in total right now that are skipped for this reason

@ChrisDryden ChrisDryden mentioned this pull request Dec 8, 2025
Closed
@github-actions
Copy link

github-actions bot commented Dec 8, 2025

GNU testsuite comparison:

Congrats! The gnu test tests/rm/rm-readdir-fail is now passing!

1 similar comment
@github-actions
Copy link

github-actions bot commented Dec 8, 2025

GNU testsuite comparison:

Congrats! The gnu test tests/rm/rm-readdir-fail is now passing!

@ChrisDryden ChrisDryden changed the title rm: example of how to override the glibc call when implementation does not match gnu coreutils exactly rm: overriding the libc call when implementation does not match gnu coreutils exactly Dec 8, 2025
@ChrisDryden ChrisDryden marked this pull request as ready for review December 9, 2025 02:36
@ChrisDryden ChrisDryden mentioned this pull request Dec 24, 2025
Closed
@sylvestre
Copy link
Contributor

sylvestre commented Dec 29, 2025

sorry, it needs to be rebased

@github-actions
Copy link

github-actions bot commented Dec 30, 2025

GNU testsuite comparison:

Congrats! The gnu test tests/rm/rm-readdir-fail is now passing!

This was referenced Jan 20, 2026
Open
Open
oech3
oech3 reviewed Jan 20, 2026
View reviewed changes
util/build-gnu.sh
# spell-checker:ignore (paths) abmon deref discrim eacces getlimits getopt ginstall inacc infloop inotify reflink ; (misc) INT_OFLOW OFLOW
# spell-checker:ignore baddecode submodules xstrtol distros ; (vars/env) SRCDIR vdir rcexp xpart dired OSTYPE ; (utils) greadlink gsed multihardlink texinfo CARGOFLAGS
# spell-checker:ignore openat TOCTOU CFLAGS tmpfs gnproc
# spell-checker:ignore hfsplus casefold chattr dirp memcpy
Copy link
Contributor

@oech3 oech3 Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is hfsplus really used?

@sylvestre
Copy link
Contributor

sylvestre commented Jan 20, 2026

need to be rebased

oech3
oech3 reviewed Jan 20, 2026
View reviewed changes
util/build-gnu.sh
# * the selinux crate is handling errors
# * the test says "maybe we should not fail when no context available"
sed -i -e "s|returns_ 1||g" tests/cp/no-ctx.sh
"${SED}" -i -e "s|returns_ 1||g" tests/cp/no-ctx.sh
Copy link
Contributor

@oech3 oech3 Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have

coreutils/util/build-gnu.sh

Lines 14 to 15 in 456252f

command -v gsed && sed(){ gsed "$@";}
SED=$(command -v gsed||command -v sed) # for find...exec...

@codspeed-hq
Copy link

codspeed-hq bot commented Jan 20, 2026

CodSpeed Performance Report

Merging this PR will degrade performance by 15.3%

Comparing ChrisDryden:example_ld_fix (5ae2096) with main (f39081d)1

Summary

⚡ 2 improved benchmarks
❌ 6 regressed benchmarks
✅ 274 untouched benchmarks
🆕 2 new benchmarks
⏩ 38 skipped benchmarks2

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
⚡ Memory du_wide_tree[(5000, 500)] 1.3 MB 1.2 MB +8.29%
❌ Memory sort_accented_data[500000] 21.6 MB 25.5 MB -15.3%
❌ Memory sort_ascii_only[500000] 21.6 MB 25.5 MB -15.29%
❌ Memory sort_numeric[500000] 44.8 MB 48.7 MB -8.01%
❌ Memory sort_mixed_data[500000] 21.9 MB 25.8 MB -15.13%
❌ Memory sort_key_field[500000] 28.9 MB 32.8 MB -11.85%
🆕 Simulation join_unicode_locale N/A 1.3 ms N/A
⚡ Memory cp_large_file[16] 120 KB 113.4 KB +5.82%
🆕 Memory join_unicode_locale N/A 57.2 KB N/A
❌ Memory dd_copy_partial 128.4 KB 132.7 KB -3.28%

Footnotes

  1. No successful run was found on main (456252f) during the generation of this report, so f39081d was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

  2. 38 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@github-actions
Copy link

github-actions bot commented Jan 20, 2026

GNU testsuite comparison:

Congrats! The gnu test tests/rm/rm-readdir-fail is now passing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@oech3 oech3 oech3 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ChrisDryden @sylvestre @oech3